Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block input bitmap rework #3236

Merged
merged 9 commits into from Feb 24, 2020

Conversation

antiochp
Copy link
Member

@antiochp antiochp commented Feb 21, 2020

Resolves #3235.
Extracted the "undo list" concept from the kernel index exploration (#3228) and took advantage of it for outputs (where undo == unspend during rewind).

This allows the output_pos index (quick lookup of output pos by output commitment) to more closely track the current utxo set in a transactional manner.
We now keep the output_pos index updated when applying new blocks and when rewinding existing blocks under a fork scenario.

This eliminates the edge-case of needing to account for "false positive" results in the index.
We still do not treat the output_pos as authoritative but there are currently believed to be no cases where it will temporarily diverge from the set of unspent output pos.

We rebuild (via optimized init) the output_pos_index on -

  1. startup, to ensure the node starts up in a consistent state
  2. compact (once outputs are removed and pruned/compacted we no longer need to keep them in the index)
  3. during fast sync, after receiving txhashset from a peer

We log discrepencies between the index and set of outputs during index rebuild.
This should give us some insight into whether our assumptions are correct here.
If we are comfortable that we can begin treating this index as authoritative then we will only need to rebuild the index on sync (3) above.

This PR is a relatively large change in terms of code changes. But conceptually it is pretty straightforward.

  • Update output_pos index when we call apply_block()
    • add new index entries for new outputs
    • remove index entries for spent outputs
  • Update output_pos on rewind (per block)
    • remove "future" outputs that no longer exist
    • re-add "unspent" entries to the index based on per-block "spent_index" (aka undo list)
  • Store a spent_index per block when we store the associated block in the db

We used to do something similar but not exactly the same -

  • we stored a bitmap of spent pos per block
    • this allowed us to rewind the utxo set itself, but not the associated index
  • we did not undo these in the index itself when rewinding
    • this led to false positives when reading the index, so we always went to the PMMR to corroborate what the index said (if index was inconsistent with PMMR then treat as spent)

The majority of the code change involves breaking the rewind functionality out to do it per iteratively block. Before we could simply take the union of all the per-block bitmaps. This has been reworked to rewind block by block to take advantage of our ability to rewind the index itself.


We were originally hoping to get the kernel_index impl into 3.1.0 but this was not possible given the timing of the 3.1.0 release. This PR gives us a way of confirming the approach works on the existing output_pos index.

@antiochp antiochp added this to the 3.1.0 milestone Feb 21, 2020
@antiochp antiochp self-assigned this Feb 21, 2020
@antiochp
Copy link
Member Author

Will rebase once #3233 has been merged to fix the croaring issue.

and reworking rewind to simply iterate over blocks, rewinding each incrementally
just use the order of the inputs in the block
chain/src/types.rs Outdated Show resolved Hide resolved
chain/src/pipe.rs Show resolved Hide resolved
chain/src/txhashset/txhashset.rs Show resolved Hide resolved
@jaspervdm
Copy link
Contributor

I think these changes behave as expected, however I am not an expert in this area of the code. Would appreciate it if we have one additional review

@antiochp
Copy link
Member Author

Making an executive decision to merge this now. Feel free to continue review post merge.
I'd like to get the initial 3.1.0-beta tagged today.

@antiochp antiochp merged commit cb2b909 into mimblewimble:master Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rework output_pos index (better txhashset transactional support)
2 participants